fix: Enhance plugin resolution in ModuleLoader #3803
Conversation
Added a check to ensure that plugin paths do not start with "file://", in addition to existing checks for relative and absolute paths. This improves the robustness of the plugin resolution logic.
There was a problem hiding this comment.
Pull request overview
This PR enhances the plugin resolution logic in ModuleLoader.ts to handle file:// URLs returned by import.meta.resolve(). The change prevents these URLs from being incorrectly resolved as node modules, addressing an issue where path.isAbsolute() returns false for file:// URLs, causing them to be treated as relative paths.
Key Changes
- Added a check for
file://protocol in plugin path validation to skip node module resolution for such URLs
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| !path.isAbsolute(plugin) && | ||
| !plugin.startsWith("file://") |
There was a problem hiding this comment.
The check !plugin.startsWith("file://") prevents file:// URLs from being resolved as node modules, but these URLs may not be usable by Prettier's plugin loader. Node.js require() expects file paths, not file:// URLs.
Consider converting file:// URLs to file paths:
if (typeof plugin === "string") {
if (plugin.startsWith("file://")) {
return require("url").fileURLToPath(plugin);
}
if (!plugin.startsWith(".") && !path.isAbsolute(plugin)) {
return resolveNodeModule(plugin, { paths: [fileName] }) || plugin;
}
}
return plugin;This would handle file:// URLs by converting them to file paths before they reach Prettier's plugin loading logic.
| !plugin.startsWith(".") && | ||
| !path.isAbsolute(plugin) | ||
| !path.isAbsolute(plugin) && | ||
| !plugin.startsWith("file://") |
There was a problem hiding this comment.
The new file:// URL handling logic lacks test coverage. Consider adding tests to verify that:
- Plugins with
file://URLs are not resolved as node modules file://URLs are properly handled and can be loaded by Prettier- The behavior works correctly in both isolated and pnpm/workspace scenarios
Example test:
test("it handles file:// URLs in plugin paths", async () => {
const config = {
plugins: ["file:///path/to/plugin"]
};
const result = resolveConfigPlugins(config, "/test/file.js");
// Assert expected behavior
});
2 participants
Added a check to ensure that plugin paths do not start with
file://, in addition to the existing checks for relative and absolute paths. This improves the robustness of the plugin resolution logic.I created a shared Prettier configuration via
npmthat includes plugins. However, in apnpmproject, the plugin strings fail to locate the corresponding modules unlessimport.meta.resolveis used to resolve the module paths (see this issue).After applying this fix, the Prettier CLI works correctly to format documents.
However,
import.meta.resolve()returns URLs that start withfile://. Sincepath.isAbsolute(plugin)returnsfalsefor such URLs, the path is incorrectly treated as non-absolute, leading to unexpected behavior. This can be resolved by converting the URL to a file path usingurl.fileURLToPath(import.meta.resolve('xxx')), which allows the plugin to work correctly with Prettier.To improve reliability, I suggest adding an explicit check to treat paths starting with
file://as absolute paths during plugin resolution.